Review: scale Postgres memory settings with instance RAM

Summary: A tightly-scoped, low-risk follow-up to #98 that adds a RAM-tiered shared_buffers / effective_cache_size / work_mem / maintenance_work_mem block to auto-scale-memory.sh and threads the values through docker-compose.yml as -c flags with conf-equivalent defaults; the approach is sound and backward-compatible.

Requirements context: No matching spec/plan found — insforge-standalone has no docs/superpowers/ (or any docs/) directory; it is the deploy/compose repo. Assessed against the PR description, linked PRs (#98 connection scaling, #99 load evidence), and the existing code.

I verified the key backward-compat claim: the compose -c defaults (shared_buffers=32MB, effective_cache_size=128MB, work_mem=2MB, maintenance_work_mem=16MB) exactly equal the values shipped in docker-init/db/postgresql.conf:29-32, and PostgreSQL command-line -c settings override config_file, so hosts without the new .env vars behave identically to today. bash -n passes. The new sed-removal keys (auto-scale-memory.sh:119) match the appended keys, so the script stays idempotent. Tier thresholds (:90-96) align exactly with the connection-scaling thresholds (:73-79) — good consistency.

Findings

Critical

(none)

Suggestion

• Functionality / performance — worst-case work_mem vs. the container cgroup limit (auto-scale-memory.sh:90-96). The postgres container's hard memory limit is POSTGRES_MEMORY ≈ 0.43 × usable host RAM (docker-compose.yml:12, base 150/350 × scale factor). At the top tiers the worst-case work_mem × max_connections approaches that ceiling: xl → 16MB × 400 ≈ 6.4GB + shared_buffers 1GB vs. a ~6.8GB container; 2xl → 16MB × 700 ≈ 11.2GB + shared_buffers 2GB vs. a ~13GB container. In practice the real bound is PGRST_DB_POOL (250/450, not 700) and the rarity of every backend sorting at full work_mem across multiple nodes at once — which is why this is a suggestion, not a blocker — but a cgroup OOM kills the postmaster (full DB crash), not just one backend. Consider noting in the comment that the pool cap (not max_connections) is the operative bound, and/or monitoring RSS at 2xl under load.
• Software engineering — no assertion of the tier→values mapping. The selection logic is now two parallel 7-branch tables; an accidental future edit to one threshold could silently mis-tune a tier. The repo has no shell-test harness (so this is not a convention violation), but a small bats/shell check asserting TOTAL_MEM → (shared_buffers, work_mem, …) for representative values would guard both this block and the #98 connection block against drift.

Information

• effective_cache_size is a planner cost hint only (no allocation), so the multi-GB values at large tiers carry no OOM risk. On a box shared with the API the chosen values (e.g. 4GB at xl, 8GB at 2xl) slightly over-estimate cache actually available to Postgres, which biases the planner toward index scans — but the values are conservative relative to host RAM, so this is fine.
• Pre-existing (not this PR): docker-init/db/postgresql.conf:35 still reads max_connections = 60 while the compose default is :-100 and tiers reach 700. The -c override supersedes the conf value at runtime, so it's cosmetic, but the stale conf comment/value could confuse future readers — worth a cleanup pass someday.

Security: No security-relevant changes — no new user input, no secrets, no auth surface; tier values are static constants and the app.encryption_key command arg is unchanged.

Verdict

approved (informational — human approval via the separate approve flow). No blocking issues; the two suggestions are non-blocking hardening/observability notes. Note the PR is currently marked draft.

LGTM - approved.

Review: scale Postgres memory with instance RAM

Summary: Two well-scoped, load-test-backed changes to auto-scale-memory.sh + a matching docker-compose.yml command — tier-scaled Postgres memory settings and a container-limit rebalance that gives Postgres the RAM the fixed-footprint Node/PostgREST containers don't use. The logic is correct and backward-compatible; my findings are non-blocking accuracy/tuning notes.

Requirements context

No /docs/superpowers/ or docs/specs/ directory exists in this repo (it's the deploy/compose repo — root has only auto-scale-memory.sh, docker-compose.yml, docker-init/, functions). No matching spec/plan found — assessing against the PR description, the referenced load test, and the surrounding code alone. Intent is clear and internally documented in the PR body and inline comments.

I re-derived the per-tier container split independently and it matches the PR body exactly (postgres share ≈ 43/43/62/81/91/95/98% across nano→2xl), so the arithmetic checks out.

Critical

(none) — no correctness, security, data-loss, or backward-compatibility defects found.

I specifically verified backward compat: the four new -c defaults in docker-compose.yml:7 (shared_buffers=32MB, effective_cache_size=128MB, work_mem=2MB, maintenance_work_mem=16MB) exactly match the shipped docker-init/db/postgresql.conf (32MB / 128MB / 2MB / 16MB). So an instance with no new env vars behaves identically until auto-scale-memory.sh re-runs — the claim holds. The re-run path is idempotent too: the new PG_* keys are added to the sed delete list (auto-scale-memory.sh:139) before being re-appended.

Suggestion

• Functionality / accuracy — stale rationale comment (auto-scale-memory.sh:106-109). The comment states shared_buffers "stays ~15-25% of the postgres CONTAINER limit (POSTGRES_MEMORY, ~35-40% of host RAM)". That describes the pre-rebalance linear split. After the rebalance introduced in this same PR, the Postgres container is 62–98% of usable RAM on small→2xl (only nano/micro remain ~43%), and shared_buffers is now ~7% of the container at the large tier (512MB / ~7.4GB), not 15-25%. Recommend updating the comment so future maintainers don't reason from the old ratios.
• Performance — shared_buffers / effective_cache_size now conservative relative to the larger container (auto-scale-memory.sh:110-116). Because the rebalance hands Postgres ~90%+ of the box, shared_buffers=512MB (large) / 2GB (2xl) is a small fraction of the container, and effective_cache_size (2GB at large on an ~8GB box) under-hints the total cache available to the planner. This is safe and load-tested (504 TPS, 0 OOM), so it's non-blocking — but there may be headroom to raise both now that the memory is actually allocated to PG. Worth a follow-up load test rather than a change here.
• Software engineering — no automated guard for the tier math (auto-scale-memory.sh). The threshold table and the min()/floor logic are easy to break silently on edit. The repo has no test harness, so this isn't a convention violation — but a lightweight --dry-run/bats check asserting the boundary values (e.g. TOTAL_MEM=7500 → shared_buffers=512MB, container ≈ USABLE−768) would protect the boundaries cheaply. Non-blocking given repo conventions.

Information

• The # Verify total doesn't exceed usable memory block (auto-scale-memory.sh:120-129, pre-existing) only echoes TOTAL_ALLOCATED; it doesn't guard or clamp. With the new Postgres floor (auto-scale-memory.sh:81) plus awk rounding, TOTAL_ALLOCATED can exceed USABLE_MEM by ~1MB on nano. Cosmetic, and unchanged in spirit from before.
• The Postgres-memory tiers and the connection-scaling/container tiers use independent TOTAL_MEM thresholds, but I checked they stay mutually consistent (e.g. shared_buffers never approaches the Postgres container limit — worst ratio is nano at ~15%), so there's no risk of a high shared_buffers on a small container.
• Security: no security-relevant changes — no new user input reaches SQL/shell, no secrets logged (the script echoes only memory sizes), and the pre-existing app.encryption_key handling in the compose command is untouched.
• This PR is marked draft; flagging in case it's not yet intended for merge.

Verdict

approved (informational; posted as a comment — explicit GitHub approval remains a separate human action). Zero Critical findings. The change is correct, backward-compatible, and backed by a documented load test; the notes above are accuracy and tuning follow-ups the author can take or leave.

LGTM - approved.

Review: scale Postgres memory with instance RAM

Summary: A well-reasoned, load-test-backed infra tuning change — Postgres config values now scale by RAM tier and the container-limit split gives Postgres the remainder instead of scaling all three services linearly; the logic is internally consistent and backward-compatible, with no blocking issues.

Requirements context

No matching spec/plan found — insforge-standalone has no docs/superpowers/ (or docs/specs/) directory, so this was assessed against the PR description and the surrounding code alone. The change is self-contained to auto-scale-memory.sh and docker-compose.yml.

Findings

Critical

(none)

Suggestion

• Functionality — CPU limit is not scaled alongside memory (docker-compose.yml:8-13). The Postgres container is pinned at cpus: '0.5' for every tier. The PR's premise is that "only Postgres benefits from more resources as the box grows," and on a large/xl/2xl box Postgres now gets ~90–98% of RAM but is still throttled to half a core. The sort-heavy load test measured TPS and OOM but not CPU saturation. Explicitly out of the stated (memory-only) scope, so non-blocking — but worth a follow-up: a memory-rich Postgres capped at 0.5 CPU may just move the bottleneck.
• Software engineering — no automated coverage for the tier logic (auto-scale-memory.sh:112-119). Consistent with the repo (no test harness exists), so not a blocker. The branch table is now the third parallel tier ladder in this file (container split, connection scaling, memory settings) that must be kept in lockstep by hand. A tiny table-driven smoke test (feed a TOTAL_MEM per tier, assert the emitted .env) would cheaply guard against a future drift where, say, a shared_buffers bump outgrows its container limit.

Information

• Backward-compat claim verified. The compose -c fallback defaults (shared_buffers=32MB, effective_cache_size=128MB, work_mem=2MB, maintenance_work_mem=16MB at docker-compose.yml:7) exactly match the shipped docker-init/db/postgresql.conf:29-32. Instances without the new env vars behave identically until auto-scale-memory.sh re-runs, as stated. ✅
• .env regeneration is idempotent. The four new keys were correctly added to the sed delete list (auto-scale-memory.sh:141), so re-running the script won't accumulate duplicate PG_* lines. ✅
• Per-tier sizing is safe. Spot-checked every tier: shared_buffers plus worst-case work_mem × PG_MAX_CONNECTIONS stays comfortably within the remainder-based Postgres container limit (e.g. large: 512MB + 8MB×250 ≈ 2.5GB inside a ~6.85GB container; 2xl: 2GB + 16MB×700 ≈ 13GB inside ~31GB). The POSTGRES_MEM floor at the original linear share keeps nano/micro unchanged. ✅
• /dev/shm sizing (not introduced here, but adjacent). docker-compose.yml sets no shm_size, so the container keeps Docker's 64MB /dev/shm. shared_buffers uses anonymous mmap and is unaffected, but Postgres parallel-query dynamic shared memory (dynamic_shared_memory_type=posix) lives in /dev/shm; on the larger tiers with higher work_mem and more parallel workers this could bind. Not caused by this PR (no parallelism settings changed) — flagging only so it's on the radar for the next tuning pass.
• Vestigial comment. # Verify total doesn't exceed usable memory (auto-scale-memory.sh:122) precedes an echo-only block with no actual verification, and since POSTGRES_MEM = USABLE_MEM − INSFORGE_MEM − POSTGREST_MEM, the total now equals usable by construction (or exceeds it by ~1MB only when the nano linear floor engages). Pre-existing, harmless.

Security

No security-relevant changes: no new user input reaches SQL/shell, no secrets logged (the echos print only tuning values), and auth/app.encryption_key handling is untouched.

Performance

This is a performance change, and a net positive — it fixes documented swap-thrashing on the large tier (Postgres 43% → 90% of the box) with load-test evidence and no OOM regressions. No hot-path or query concerns introduced.

Verdict

approved (informational — human approval via the separate approve flow). No Critical findings; the two Suggestions and the Information notes are all non-blocking follow-ups.

LGTM - approved.

Actionable comments posted: 1

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@auto-scale-memory.sh`:
- Around line 79-81: Update the PostgreSQL allocation logic around
POSTGRES_LINEAR and POSTGRES_MEM so the rounded PostgreSQL floor cannot make
INSFORGE_MEM + POSTGREST_MEM + POSTGRES_MEM exceed USABLE_MEM. Cap the
PostgreSQL allocation at the remaining usable memory while preserving the
existing linear-floor minimum whenever it fits.

Review: scale Postgres memory (config + container share) with instance RAM

Summary: A well-reasoned, load-test-validated change that tiers Postgres memory settings to instance RAM and rebalances container limits so Postgres — the only service that scales with the box — gets the remainder; backward compatibility is preserved and I found no blocking issues.

Requirements context

No docs/superpowers/ (or docs/specs/) directory exists in this repo — it's a minimal deployment repo (auto-scale-memory.sh, docker-compose.yml, docker-init/, functions/), with no .github/ CI or test harness. No matching spec/plan found — assessing against the PR description, the load-test evidence in the PR body, and the referenced #99 config.

Findings

Critical

(none)

Suggestion

• Functionality / Performance — effective_cache_size is conservative (~25% of box) at every tier (auto-scale-memory.sh:112-118). It's set to a fixed 4× shared_buffers (32→128MB, 512MB→2GB, 2GB→8GB), i.e. ~25% of total RAM per tier. After this PR's rebalance Postgres owns ~90–98% of a large/xl/2xl box, so the planner is being told far less OS cache is available than actually is. effective_cache_size is a planner cost hint, not an allocation (no OOM risk), and understating it biases the planner away from index scans toward seq scans/bitmap heap scans. Common guidance is ~50–75% of the memory available to Postgres. Unlike shared_buffers (deliberately conservative pending a follow-up load test, which I agree with), raising effective_cache_size has no downside, so it's a safe win to include or note for the follow-up. Non-blocking.

• Functionality — worst-case work_mem accounting under-counts hash memory (auto-scale-memory.sh:110-111,112-118). The comment bounds work_mem against work_mem * max_connections, but PG15's hash_mem_multiplier defaults to 2.0, so hash-based nodes may use up to 2 × work_mem each, and a single query can have several sort/hash nodes. The realistic worst case is thus a few× higher than work_mem × max_connections. The author explicitly load-tested sort-heavy load at the large tier with 0 OOM kills and flags tuning as a documented follow-up, so this is acceptable as shipped — noting it so the follow-up load test accounts for the multiplier and multi-node queries.

Information

• Software engineering — no automated test. Expected: this repo has no shell/compose test harness or CI (.github/ absent), and the change is validated via the documented throwaway-t4g.large load test in the PR body (before/after container limits, TPS, OOM counts). That's the appropriate validation path for this repo; not flagging as a gap.
• Backward compatibility verified. The new compose -c defaults — shared_buffers=${PG_SHARED_BUFFERS:-32MB}, effective_cache_size=…:-128MB, work_mem=…:-2MB, maintenance_work_mem=…:-16MB (docker-compose.yml:7) — exactly match the shipped docker-init/db/postgresql.conf:29-32. Instances without the new env vars behave identically until auto-scale-memory.sh re-runs, as the PR claims. ✓
• No startup-allocation OOM risk. Traced nano→2xl: shared_buffers stays ≤ ~17% of the computed POSTGRES_MEM container limit at every tier (tightest is micro at ~64MB/~395MB), so the upfront shared-memory allocation always fits the cgroup limit. ✓
• .env re-run hygiene is correct. The four new PG_* keys are added to the sed delete list (auto-scale-memory.sh:141) before the block is re-appended, so repeated runs don't accumulate stale duplicates. ✓
• Verify block is echo-only (auto-scale-memory.sh:122-131). TOTAL_ALLOCATED is computed and printed but not asserted against USABLE_MEM. By construction POSTGRES_MEM = USABLE_MEM − INSFORGE_MEM − POSTGREST_MEM, so the only way to exceed usable is the small POSTGRES_LINEAR rounding floor (:81), which is negligible. Pre-existing structure; a hard guard/exit here would make the invariant self-checking, but not required.
• Security — no security-relevant changes. No new user-controlled input reaches the shell/SQL; all -c values are fixed literals generated by the script. The app.encryption_key line is unchanged, and nothing secret is newly logged.

Verdict

approved — zero Critical findings. Two non-blocking Suggestions (raise/re-note effective_cache_size; account for hash_mem_multiplier in the follow-up tuning). The rebalance logic, tier thresholds (mirroring the existing connection-scaling block), and backward-compat defaults all check out. (Informational verdict — explicit GitHub approval remains a separate human action.)

LGTM - approved.

2 issues found across 2 files

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="auto-scale-memory.sh">

<violation number="1" location="auto-scale-memory.sh:80">
P2: Near the minimum supported RAM sizes, rounding the linear values up makes the generated limits exceed `USABLE_MEM`, consuming part of the system reservation. Keep the remainder allocation when it is smaller than the rounded linear estimate.</violation>

<violation number="2" location="auto-scale-memory.sh:112">
P1: Concurrent complex queries can exceed the PostgreSQL cgroup limit and be OOM-killed at the configured connection limits. Size `PG_WORK_MEM` from a bounded concurrent-query budget (and cap `autovacuum_work_mem`) rather than assigning 16MB to every potentially active backend.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>